Skip to content

NLog Target support ConnectionString with isolated TelemetryClient#2897

Closed
snakefoot wants to merge 7 commits intomicrosoft:mainfrom
snakefoot:b
Closed

NLog Target support ConnectionString with isolated TelemetryClient#2897
snakefoot wants to merge 7 commits intomicrosoft:mainfrom
snakefoot:b

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Aug 13, 2024

Resolves #2714 - Add support for connection string to Microsoft.ApplicationInsights.NLogTarget

Changes

  • Added ConnectionString option for NLog Target with isolated TelemetryClient (instead of modifying global config)
  • Changed NLog Target Flush to use FlushAsync (instead of blocking)

Checklist

  • I ran Unit Tests locally.
  • CHANGELOG.md updated with one line description of the fix, and a link to the original issue if available.

For significant contributions please make sure you have completed the following items:

  • Design discussion issue #
  • Changes in public surface reviewed

The PR will trigger build, unit tests, and functional tests automatically. Please follow these instructions to build and test locally.

Notes for authors:

  • FxCop and other analyzers will fail the build. To see these errors yourself, compile localy using the Release configuration.

@snakefoot snakefoot changed the title NLog Target support ConnectionString for isolated NLog Target support ConnectionString for isolated TelemetryClient Aug 13, 2024
@snakefoot snakefoot changed the title NLog Target support ConnectionString for isolated TelemetryClient NLog Target support ConnectionString with isolated TelemetryClient Aug 13, 2024
@snakefoot snakefoot force-pushed the b branch 18 times, most recently from 3e910a9 to df34463 Compare August 13, 2024 23:27
@snakefoot
Copy link
Contributor Author

snakefoot commented Aug 13, 2024

@cijothomas + @rajkumar-rangaraj + @TimothyMothra Fork of #2858 where I have changed to use isolated TelemetryConfiguration so not modifying the global config, when using ConnectionString.

I don't mind also fixing log4net-appender if you like. Thus moving away from using InstrumentationKey before it is fully deprecated.

Please review and merge.

@SharePointX
Copy link

With all due respect, we are eagerly awaiting this change as the deadline for discontinuing instrumentation keys is approaching.

@snakefoot
Copy link
Contributor Author

snakefoot commented Jan 21, 2025

@TimothyMothra Polite poke for review. Notice I don't mind updating log4net-appender as well (in new pull-request to resolve #2926)

@sievajetnamdar
Copy link

Hi @TimothyMothra can we have a review on this please?

@jcoenen96
Copy link

@TimothyMothra politely asking for a review.

I have been looking to use OpenTelemetry.Instrumentation.AspNet instead but unfortunately this package is still in Beta. Would really be appreciated if the connectionstring fix will be included in the NLog package, at least untill OpenTelemetry for ASP.Net has been fully released.

@Mek7
Copy link

Mek7 commented May 30, 2025

Now that instrumentation keys have been deprecated, I wanted to move to connection strings, but found out that NLog target does not support connection strings, only instrumentation keys.
Why is it always so that MS deprecates something but neglects to update its own tooling?

@Mek7
Copy link

Mek7 commented Jul 4, 2025

Still nothing? Is there nobody else to review this PR?
It needs some love and attention, please!

@xiang17
Copy link
Member

xiang17 commented Jul 22, 2025

The general direction for this repository is that we are not adding any new fixes/features unless there is special reason like security etc. The future direction is the OpenTelemetry SDK and we will work on closing the gaps.

@snakefoot
Copy link
Contributor Author

snakefoot commented Jul 28, 2025

@xiang17 + @cijothomas + @rajkumar-rangaraj + @TimothyMothra Yes it could be awesome if the OpenTelemetry-Logging-API became public, so one could build bridges from NLog / Log4net / Serilog into the OpenTelemetry-Exporter-API. But sadly enough it has not been prioritized.

Please prioritze merging this pull-request, since the ConnectionString is an existing feature. Believe this is more a bugfix, as the legacy InstrumentationKey is a dead end (Microsoft have burned the bridge before new is ready)

@snakefoot
Copy link
Contributor Author

I guess when #3001 is merged from develop-branch to main-branch and released, then this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for connection string to Microsoft.ApplicationInsights.NLogTarget

8 participants